fix(web): preserve node progress on Play resume + drill-back by naorsabag · Pull Request #96 · naorsabag/openhop

naorsabag · 2026-05-09T19:13:13Z

Two related bugs, same root: cumulative state (per-node progress + create/destroy) wasn't re-derived from the step index on key transitions.

What's fixed

1. Click-then-Play kept stale progress

Clicking a node fires its next outgoing step manually (good). But that increment lives in nodeProgressRef, which autoplay also uses. So:

Pause mid-step. Click node X → X's progress bar advances by 1.
Press Play → autoplay resumes from where it was. X's bar now permanently ahead of autoplay's expectation; gets pushed further every time autoplay fires X.

2. Drill-down → Back wiped parent's progress

<FlowCanvasInner> is keyed on the node-id list, so drilling into a sub-flow remounts the inner component on return — fresh nodeProgressRef = new Map(). Even though resumeFromStep was correctly seeded into stepIndexRef, the per-node bars all snapped to zero.

Implementation

Extract the inline cumulative-replay logic from goToStep into a shared applyEffectsThroughStep(idx) helper.
New mount-time useEffect: if stepIndexRef.current >= 0 on first render (i.e., startFromStep was set), replay 0..stepIndexRef and push into state. Fixes Compose roads from a single line #2.
Play-resume branch in the [playing] effect now also replays before re-entering the phase. Fixes fix: tighten road endpoints against node art #1.
goToStep refactored to use the same helper — no behavior change.

Verification

npm run typecheck — clean
npm test — 40/40
npm run format:check — clean

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Fixed animation playback to properly replay state when resuming mid-phase.
- Improved animation consistency when starting from custom positions.
- Manual scrubbing now applies the same cumulative effects as autoplay mode.

Two related bugs that share the same root: cumulative state (per- node progress, create/destroy) wasn't re-derived from the step index on key transitions. 1. Click a node mid-pause → that node's outgoing-step counter increments (handleNodeClick → setNodeStep). Press Play → autoplay resumes from where it was, but the node's progress bar stays ahead of where autoplay would actually be. 2. Drill into a sub-flow → click Back → parent flow's progress bars reset to zero. FlowCanvasInner's key includes node ids, so the parent component remounts fresh, dropping nodeProgressRef even though stepIndexRef was correctly seeded from resumeFromStep. Both fixed by extracting the inline cumulative-replay logic from goToStep into `applyEffectsThroughStep(idx)` and calling it from two new sites: - Mount-time effect: when stepIndexRef.current >= 0 on first render (i.e., startFromStep was set), replay 0..stepIndexRef and push into state so the parent flow's progress survives drill-back. - Play-resume branch in the [playing] effect: replay 0..stepIndexRef before re-entering the phase, so any manual node clicks during the pause get snapped back to autoplay-expected state. goToStep also refactored to use the same helper (no behavior change).

coderabbitai · 2026-05-09T19:13:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 47eb6f23-9eb7-4f76-ab51-9c190f520355

📥 Commits

Reviewing files that changed from the base of the PR and between dc7abd7 and bc2c90d.

📒 Files selected for processing (1)

packages/web/src/hooks/useFlowAnimation.ts

Walkthrough

The PR refactors useFlowAnimation to centralize cumulative step effect replay through a new applyEffectsThroughStep helper function. This helper is now invoked during play-resume mid-phase, mount initialization with a non-zero startFromStep, and manual step navigation, ensuring consistent node progress and visibility state reconstruction across all entry points.

Changes

Cumulative Step Effects Replay

Layer / File(s)	Summary
Cumulative Effects Helper `packages/web/src/hooks/useFlowAnimation.ts`	Introduces `applyEffectsThroughStep(idx)` to reconstruct `nodeProgress`, `activeNodes`, and `destroyedNodes` for steps `0..idx`. Includes mount-only effect that replays when `startFromStep > 0` so initial state matches autoplay expectations.
Play-Resume Integration `packages/web/src/hooks/useFlowAnimation.ts`	On `playing` resume with an active phase, cumulative effects through current `stepIndexRef` are replayed before re-entering React state, ensuring node/edge visibility is consistent with resumed playback.
goToStep Refactoring `packages/web/src/hooks/useFlowAnimation.ts`	`goToStep(idx)` now calls shared `applyEffectsThroughStep(idx)` instead of duplicating cumulative create/destroy and per-node progress logic. Dependency list updated to include `applyEffectsThroughStep`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

naorsabag/openhop#93: Both PRs modify the same hook with cumulative step effect replay and goToStep refactoring.
naorsabag/openhop#90: Both PRs modify useFlowAnimation to change phase-aware timing and cumulative step effect application.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR objectives address fixing node progress preservation on Play resume and drill-back, but the linked issues (`#1`, `#2`) concern road rendering composition and node handle positioning, which are unrelated coding requirements.	Verify that the linked issues are correct. If issues `#1` and `#2` are unrelated, remove them and link to issues that define the node progress fix requirements instead.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(web): preserve node progress on Play resume + drill-back' clearly and specifically describes the main fix: preventing loss of node progress when resuming playback and returning from nested flows.
Out of Scope Changes check	✅ Passed	Changes are focused on useFlowAnimation.ts and specifically address node progress replay logic, mount-time state restoration, and Play-resume reconciliation—all directly related to the PR's stated objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/preserve-progress-on-resume-and-drillback

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

* chore: bump @openhop/web 0.1.0-beta.1 → 0.1.0-beta.2 and openhop CLI 0.1.0-beta.3 → 0.1.0-beta.4 Web bumps for the canvas/UI work since beta.1: - pause behavior fix + label overflow + back-edge corridor (#90) - carrot click opens inspect with multi-target highlight + on-canvas playback controls (#93) - bookmark-style toggle tabs for sidebar + inspector (#95) - preserve node progress on Play resume + drill-back (#96) - shrink sprite SVGs by 46% (3.7MB → 2.0MB) (#97) CLI bumps to ship the new web bundle (its only change is the pinned @openhop/web dependency moving to 0.1.0-beta.2). @openhop/server stays at 0.1.0-beta.1 — no changes since last publish. * fix(cli): bump hardcoded --version to 0.1.0-beta.4 Test contract.test.ts asserts `openhop --version` matches the package.json version. The bump in 9b373aa missed the duplicate hardcoded string in src/index.ts:53. Sync them up so CI passes.

naorsabag merged commit 28bfc8e into master May 9, 2026
7 checks passed

naorsabag mentioned this pull request May 9, 2026

chore: bump @openhop/web to beta.2 + openhop CLI to beta.4 #98

Merged

naorsabag deleted the fix/preserve-progress-on-resume-and-drillback branch May 10, 2026 17:38

coderabbitai Bot mentioned this pull request May 11, 2026

feat(web): tune carrot dwell + cap max zoom #134

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(web): preserve node progress on Play resume + drill-back#96

fix(web): preserve node progress on Play resume + drill-back#96
naorsabag merged 1 commit into
masterfrom
fix/preserve-progress-on-resume-and-drillback

naorsabag commented May 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 9, 2026 •

edited

Loading

❌ Failed checks (2 warnings)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

naorsabag commented May 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's fixed

1. Click-then-Play kept stale progress

2. Drill-down → Back wiped parent's progress

Implementation

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (2 warnings)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

naorsabag commented May 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 9, 2026 •

edited

Loading